feat: Make add piece idempotent #8160
Conversation
extern/storage-sealing/input.go
Outdated
| }, 1) | ||
|
|
||
| m.pendingPieces[proposalCID(deal)] = &pendingPiece{ | ||
| doneCh := make(chan struct{}, 1) |
There was a problem hiding this comment.
| doneCh := make(chan struct{}, 1) | |
| doneCh := make(chan struct{}) |
There was a problem hiding this comment.
Let's keep this for safety, dosen't hurt because the code that writes to this is called sync by the sealer
There was a problem hiding this comment.
We never really write to this channel, just close it, which doesn't block.
extern/storage-sealing/sealing.go
Outdated
| type pendingPiece struct { | ||
| doneCh chan struct{} | ||
| resp atomic.Value |
There was a problem hiding this comment.
I don't think resp needs to be an atomic value - the doneCh prevents concurrent access
There was a problem hiding this comment.
good point. fixed.
Codecov Report
@@ Coverage Diff @@
## master #8160 +/- ##
==========================================
- Coverage 39.49% 39.42% -0.07%
==========================================
Files 666 666
Lines 72516 72519 +3
==========================================
- Hits 28637 28589 -48
- Misses 38932 38961 +29
- Partials 4947 4969 +22
Continue to review full report at Codecov.
|
extern/storage-sealing/input.go
Outdated
| select { | ||
| case <-pp.doneCh: | ||
| res := pp.resp | ||
| return api.SectorOffset{Sector: res.sn, Offset: res.offset.Padded()}, res.err | ||
| case <-ctx.Done(): | ||
| return api.SectorOffset{}, ctx.Err() | ||
| } |
There was a problem hiding this comment.
Could put this into a helper method (e.g. waitResp) on pendingPiece for readability, and dedupe the same bit of code below.
extern/storage-sealing/input.go
Outdated
| }, 1) | ||
|
|
||
| m.pendingPieces[proposalCID(deal)] = &pendingPiece{ | ||
| doneCh := make(chan struct{}, 1) |
There was a problem hiding this comment.
We never really write to this channel, just close it, which doesn't block.
|
@magik6k Have addressed your review. |
|
Please update the pr title as instructed in the pr template before merging |
Related Issues
filecoin-project/boost#282
Proposed Changes
A very common deal making error we encounter is that Boost/Markets makes a call to
AddPieceand then restarts.If it restarts before it's able to persist the fact that it's already called AddPiece, it will call
AddPieceagain for the same piece. In this case, the existing API returns an error as it's not idempotent.This PR allows the user to call
AddPiecemultiple times for the same piece but ensures that it's handed off to the sealing subsystem only once.It also makes the method respect the given context.
Additional Info
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>fix: mempool: Introduce a cache for valid signaturesPR type: fix, feat, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, testarea: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps